-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update support for weak references #2248
Conversation
This commit updates the `WASM_BINDGEN_WEAKREF` feature support to the latest version of the weak references proposal in JS. This also updates the `Closure` type to use finalization registries to ensure closures are deallocated with the JS gc. This means that `Closure::forget` will not actually leak memory in a weakref-using application.
/// will not leak memory. Instead the Rust memory will be reclaimed when the | ||
/// JS closure is GC'd. Weak references are not enabled by default since | ||
/// they're still a proposal for the JS standard. They can be enabled with | ||
/// `WASM_BINDGEN_WEAKREF=1` when running `wasm-bindgen`, however. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe time to make an actual CLI flag, now that these are pretty far down the standards pipeline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it's worth turning this on by default and requiring a flag to turn it off? I'm not actually sure at what point we should do that...
I agree though that at least this should be a CLI flag now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we are at default-on yet. Probably not for a year or so, I'd guess, if ever :-/
Just a heads up, this change broke the |
Thanks for the report! I'll publish a new release with #2258 to fix this. |
This commit updates the
WASM_BINDGEN_WEAKREF
feature support to thelatest version of the weak references proposal in JS. This also updates
the
Closure
type to use finalization registries to ensure closures aredeallocated with the JS gc. This means that
Closure::forget
will notactually leak memory in a weakref-using application.